-
Notifications
You must be signed in to change notification settings - Fork 5.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
executor: only show valid columns in stats_histogram
#9487
Conversation
/run-all-tests |
Codecov Report
@@ Coverage Diff @@
## master #9487 +/- ##
==========================================
- Coverage 67.42% 67.41% -0.02%
==========================================
Files 373 373
Lines 78568 78570 +2
==========================================
- Hits 52976 52965 -11
- Misses 20837 20845 +8
- Partials 4755 4760 +5
Continue to review full report at Codecov.
|
@@ -82,6 +82,9 @@ func (e *ShowExec) appendTableForStatsHistograms(dbName, tblName, partitionName | |||
return | |||
} | |||
for _, col := range statsTbl.Columns { | |||
if col.IsInvalid(nil, false) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not pass sc here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to mark column stats needed in show stats_histograms
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how about adding a comment like "pass a nil StatementContext to avoid column stats being marked as needed"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment added, PTAL
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
What problem does this PR solve?
Fix #9479
What is changed and how it works?
Check if the column is valid or loaded before appending it to result of
show stats_histograms
.Check List
Tests
Code changes
N/A
Side effects
N/A
Related changes